Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-93274: Disable Limited API tests with Py_TRACE_REFS #95796

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

encukou
Copy link
Member

@encukou encukou commented Aug 8, 2022

This should fix the Py_TRACE_REFS buildbot.

(In the future we'll need a better way to write limited API tests.)

@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 8, 2022
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @encukou for commit 430a940 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 8, 2022
@@ -1,3 +1,16 @@
#include "pyconfig.h" // Py_TRACE_REFS

#ifdef Py_TRACE_REFS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to move this #ifdef in _testcapimodule.c instead.

// Py_TRACE_REFS is incompatible with Limited API
#include "parts.h"
int
_PyTestCapi_Init_VectorcallLimited(PyObject *m) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest putting an #ifdef on the _PyTestCapi_Init_VectorcallLimited() call in _testcapimodole.c instead of defining a no-op function.

Maybe define somewhere a macro to decide if vectorcall is tested or not? Something like:

#ifndef Py_TRACE_REFS
#  define TEST_VECTORCALL
#endif

@encukou
Copy link
Member Author

encukou commented Aug 8, 2022

If the tests pass, I'll merge this as is and make it better later. I'd like to make testing limited API as smooth as possible, which'll be much easier without the pressure of a red buildbot.
I'll definitely take the comments into account!

@encukou
Copy link
Member Author

encukou commented Aug 15, 2022

@vstinner: #95991 includes your suggestions (with changes – the check is in parts.h rather than _testcapimodule.c, and the macro is LIMITED_API_AVAILABLE rather than TEST_VECTORCALL.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants